Conversation
* refactor: remove deprecated async utility from @angular/core/testing * refactor: remove deprecated async utility from @angular/core/testing * feat(ui): ui-plugin upwork integration module for integration plugins * feat(ui): ui-plugin upwork integration module for integration plugins * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers * fix(build): apply AI suggestion by bot agent reviewers
* fix: more security focused fixes * fix: add env var to all relevant places * fix: multiple ORMs * Update packages/config/src/lib/environments/environment.prod.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update packages/core/src/lib/auth/email-confirmation.service.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: more * chore: spelling * fix: more * fix: regex --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| helmet({ | ||
| contentSecurityPolicy: isProduction | ||
| ? undefined // use Helmet's strict default CSP in production | ||
| : false // disable CSP in dev/stage so Swagger/Scalar inline scripts work | ||
| }) |
Check failure
Code scanning / CodeQL
Insecure configuration of Helmet security middleware High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 25 days ago
Generally, the fix is to stop disabling Helmet’s contentSecurityPolicy entirely and instead configure CSP in a way that both (a) remains enabled in all environments and (b) is relaxed enough in non-production to allow Swagger/Scalar UI to function. That means: remove contentSecurityPolicy: false and replace it with a custom CSP configuration for non-production, while keeping the strict/standard defaults for production.
Concretely, in packages/core/src/lib/bootstrap/index.ts, lines 168–175, we should change the Helmet setup to always have CSP enabled. For production (isProduction === true), we keep the default Helmet behaviour by not passing a contentSecurityPolicy option. For non-production, we pass an explicit contentSecurityPolicy configuration whose directives extend Helmet’s defaults and loosen only what is needed for Swagger/Scalar: typically allowing inline scripts ('unsafe-inline') and the app’s own origin in script-src, and possibly adjusting connect-src to allow the API host. To avoid guessing too many details, we can (1) use helmet.contentSecurityPolicy.getDefaultDirectives() to start from Helmet’s defaults, (2) spread those defaults into a new directives object, and (3) override only a couple of keys such as "script-src" for non-production. This preserves existing functionality and keeps CSP protection in place everywhere, while still allowing Swagger/Scalar to work.
To implement this, we need to: (1) update the Helmet import to also bring in the named contentSecurityPolicy helper, and (2) change the app.use(helmet({ ... })) block to conditionally add contentSecurityPolicy only in non-production, using contentSecurityPolicy.getDefaultDirectives() to build the directives. No new external dependencies are required, only using what Helmet already exposes.
| @@ -11,6 +11,7 @@ | ||
| // Note: below code can't be moved to other places because it has to be executed first, before we load any other modules! | ||
|
|
||
| import tracer from './tracer'; | ||
| import helmet, { contentSecurityPolicy } from 'helmet'; | ||
|
|
||
| /** | ||
| * Start tracing using if OTEL is enabled. | ||
| @@ -165,15 +166,24 @@ | ||
| // Configure Redis or in-memory sessions | ||
| await configureRedisSession(app); | ||
|
|
||
| // Use helmet for security headers — relax CSP in non-production to avoid blocking Swagger/Scalar UI | ||
| app.use( | ||
| helmet({ | ||
| contentSecurityPolicy: isProduction | ||
| ? undefined // use Helmet's strict default CSP in production | ||
| : false // disable CSP in dev/stage so Swagger/Scalar inline scripts work | ||
| }) | ||
| ); | ||
| // Use helmet for security headers — keep CSP enabled, but relax it slightly in non-production to avoid blocking Swagger/Scalar UI | ||
| const helmetOptions: Parameters<typeof helmet>[0] = {}; | ||
|
|
||
| if (!isProduction) { | ||
| const defaultDirectives = contentSecurityPolicy.getDefaultDirectives(); | ||
|
|
||
| helmetOptions.contentSecurityPolicy = { | ||
| directives: { | ||
| ...defaultDirectives, | ||
| // Allow inline scripts in non-production for tools like Swagger/Scalar UI. | ||
| // Adjust this further if you host Swagger/Scalar from a different origin. | ||
| "script-src": ["'self'", "'unsafe-inline'"], | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| app.use(helmet(helmetOptions)); | ||
|
|
||
| // Set the global prefix for routes | ||
| const globalPrefix = 'api'; | ||
| app.setGlobalPrefix(globalPrefix); |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 7421875 | Triggered | Google API Key | b70bc3d | .deploy/k8s/k8s-manifest.demo.yaml | View secret |
| 7461871 | Triggered | Generic High Entropy Secret | b70bc3d | .deploy/k8s/k8s-manifest.demo.yaml | View secret |
| 7421875 | Triggered | Google API Key | b70bc3d | .deploy/k8s/k8s-manifest.stage.yaml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Too many files changed for review. ( |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38f54cf7f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Invalidate the magic code after first successful use | ||
| switch (this.ormType) { | ||
| case MultiORMEnum.MikroORM: | ||
| await this.mikroOrmUserRepository.nativeUpdate( | ||
| { email, code }, | ||
| { code: null, codeExpireAt: null } | ||
| ); | ||
| break; | ||
| case MultiORMEnum.TypeORM: | ||
| await this.typeOrmUserRepository.update({ email, code }, { code: null, codeExpireAt: null }); |
There was a problem hiding this comment.
Delay magic-code invalidation until workspace signin completes
This now clears code/codeExpireAt immediately after /signin.email/confirm, but the returned workspace tokens are generated from that same code (createUserSigninWorkspaceResponse → generateToken) and workspaceSigninVerifyToken still requires the stored code and codeExpireAt to match. In the normal email-code flow, the next /signin.workspace step will fail as unauthorized because the code was already nulled out. The invalidation should happen after successful workspaceSigninVerifyToken, not before issuing workspace tokens.
Useful? React with 👍 / 👎.
| * Set to 8 characters for security (36^8 ≈ 2.8 trillion combinations). | ||
| */ | ||
| export const ALPHA_NUMERIC_CODE_LENGTH = 6; | ||
| export const ALPHA_NUMERIC_CODE_LENGTH = 8; |
There was a problem hiding this comment.
Keep login code length compatible across clients and defaults
Changing the shared code length to 8 makes backend validation expect 8-character codes, but existing clients still enforce 6-character input (for example packages/ui-auth/src/lib/components/login-magic/login-magic.component.ts:41, packages/plugins/public-layout-ui/src/lib/components/workspace-actions/shared/base-workspace-auth.component.ts:45, and packages/desktop-ui-lib/src/lib/login/features/login-magic/login-magic.component.ts:62), and demo credentials still default to 123456 in config. This causes real/demo magic-code sign-in submissions to be rejected after this commit unless all producers/consumers are updated together.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3 issues found across 173 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/desktop-timer-app-stage.yml">
<violation number="1" location=".github/workflows/desktop-timer-app-stage.yml:428">
P2: Building PATH with an empty `$yarnPath` inserts an empty segment (`;;`), which on Windows resolves to the current working directory. This unintentionally adds the workspace to PATH and can lead to executing unintended binaries. Filter out empty entries before joining.</violation>
</file>
<file name="packages/core/src/lib/auth/email-confirmation.service.ts">
<violation number="1" location="packages/core/src/lib/auth/email-confirmation.service.ts:167">
P1: TOCTOU race condition: The comment claims the update "scopes by id + code + codeExpireAt" but the actual `update(user['id'], ...)` call only scopes by `id`. Two concurrent requests can both read the same valid code, both pass validation, and both proceed — the verification code reuse is not actually prevented.
To fix this properly, the UPDATE's WHERE clause must include `code` and `codeExpireAt` (not just `id`), and you need to check that `affected` rows > 0. For example, using a query builder:
```typescript
const result = await getManager()
.createQueryBuilder()
.update(User)
.set({ code: null, codeExpireAt: null })
.where({ id: user['id'], code, codeExpireAt: MoreThanOrEqual(new Date()) })
.execute();
if (!result.affected) {
throw new BadRequestException('Verification code already used or expired.');
}
```</violation>
</file>
<file name="packages/config/src/lib/environments/environment.prod.ts">
<violation number="1" location="packages/config/src/lib/environments/environment.prod.ts:66">
P1: Calling `validateProductionSecrets()` at module load time in `environment.prod.ts` will execute in the browser bundle as part of the Angular prod environment. Since browser builds don’t have backend JWT secrets, this throws and breaks the production frontend. Guard this so it only runs on the server (or move the validation to backend startup).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Atomically invalidate the verification code (prevent reuse / TOCTOU race) // cspell:ignore TOCTOU | ||
| // The update scopes by id + code + codeExpireAt so a concurrent request | ||
| // that already nullified the code will update zero rows | ||
| await this.userService.update(user['id'], { |
There was a problem hiding this comment.
P1: TOCTOU race condition: The comment claims the update "scopes by id + code + codeExpireAt" but the actual update(user['id'], ...) call only scopes by id. Two concurrent requests can both read the same valid code, both pass validation, and both proceed — the verification code reuse is not actually prevented.
To fix this properly, the UPDATE's WHERE clause must include code and codeExpireAt (not just id), and you need to check that affected rows > 0. For example, using a query builder:
const result = await getManager()
.createQueryBuilder()
.update(User)
.set({ code: null, codeExpireAt: null })
.where({ id: user['id'], code, codeExpireAt: MoreThanOrEqual(new Date()) })
.execute();
if (!result.affected) {
throw new BadRequestException('Verification code already used or expired.');
}Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/lib/auth/email-confirmation.service.ts, line 167:
<comment>TOCTOU race condition: The comment claims the update "scopes by id + code + codeExpireAt" but the actual `update(user['id'], ...)` call only scopes by `id`. Two concurrent requests can both read the same valid code, both pass validation, and both proceed — the verification code reuse is not actually prevented.
To fix this properly, the UPDATE's WHERE clause must include `code` and `codeExpireAt` (not just `id`), and you need to check that `affected` rows > 0. For example, using a query builder:
```typescript
const result = await getManager()
.createQueryBuilder()
.update(User)
.set({ code: null, codeExpireAt: null })
.where({ id: user['id'], code, codeExpireAt: MoreThanOrEqual(new Date()) })
.execute();
if (!result.affected) {
throw new BadRequestException('Verification code already used or expired.');
}
```</comment>
<file context>
@@ -150,26 +148,27 @@ export class EmailConfirmationService {
+ // Atomically invalidate the verification code (prevent reuse / TOCTOU race) // cspell:ignore TOCTOU
+ // The update scopes by id + code + codeExpireAt so a concurrent request
+ // that already nullified the code will update zero rows
+ await this.userService.update(user['id'], {
+ code: null,
+ codeExpireAt: null
</file context>
| } | ||
|
|
||
| // Validate JWT secrets at module load time (production only) | ||
| validateProductionSecrets(); |
There was a problem hiding this comment.
P1: Calling validateProductionSecrets() at module load time in environment.prod.ts will execute in the browser bundle as part of the Angular prod environment. Since browser builds don’t have backend JWT secrets, this throws and breaks the production frontend. Guard this so it only runs on the server (or move the validation to backend startup).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/config/src/lib/environments/environment.prod.ts, line 66:
<comment>Calling `validateProductionSecrets()` at module load time in `environment.prod.ts` will execute in the browser bundle as part of the Angular prod environment. Since browser builds don’t have backend JWT secrets, this throws and breaks the production frontend. Guard this so it only runs on the server (or move the validation to backend startup).</comment>
<file context>
@@ -11,6 +11,60 @@ import { FileStorageProviderEnum } from '@gauzy/contracts';
+}
+
+// Validate JWT secrets at module load time (production only)
+validateProductionSecrets();
+
if (process.env.IS_ELECTRON && process.env.GAUZY_USER_PATH) {
</file context>
| validateProductionSecrets(); | |
| if (typeof window === 'undefined') { | |
| validateProductionSecrets(); | |
| } |
| $localNodeExe = Join-Path $localBin "node.exe" | ||
| if (-not (Test-Path $localNodeExe)) { Copy-Item $nodeExe $localNodeExe -Force } | ||
|
|
||
| $newPath = "$nodePath;$npmGlobalBin;$localBin;$yarnPath;$($env:PATH)" |
There was a problem hiding this comment.
P2: Building PATH with an empty $yarnPath inserts an empty segment (;;), which on Windows resolves to the current working directory. This unintentionally adds the workspace to PATH and can lead to executing unintended binaries. Filter out empty entries before joining.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/desktop-timer-app-stage.yml, line 428:
<comment>Building PATH with an empty `$yarnPath` inserts an empty segment (`;;`), which on Windows resolves to the current working directory. This unintentionally adds the workspace to PATH and can lead to executing unintended binaries. Filter out empty entries before joining.</comment>
<file context>
@@ -408,18 +408,35 @@ jobs:
+ $localNodeExe = Join-Path $localBin "node.exe"
+ if (-not (Test-Path $localNodeExe)) { Copy-Item $nodeExe $localNodeExe -Force }
+
+ $newPath = "$nodePath;$npmGlobalBin;$localBin;$yarnPath;$($env:PATH)"
+ "PATH=$newPath" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8
+
</file context>
|
View your CI Pipeline Execution ↗ for commit 38f54cf
☁️ Nx Cloud last updated this comment at |



PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by cubic
Strengthened backend security and deployment defaults (CORS, JWT secret validation, secure k8s settings) and improved CI reliability on Windows. Also enabled the Upwork integration via the plugin-based UI.
Security
DevOps
Written for commit 38f54cf. Summary will update on new commits.